Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seek to beginning of IOBuffer before returning it #212

Closed
wants to merge 1 commit into from

Conversation

ikirill
Copy link

@ikirill ikirill commented Nov 9, 2021

It’s impossible to read from an IOBuffer that was just written to: its
position is set to its end and reading from it results in EOFError.

It’s impossible to read from an IOBuffer that was just written to: its
position is set to its end and reading from it results in EOFError.
@ViralBShah
Copy link
Contributor

@ikirill Do we need to merge this?

@vtjnash
Copy link
Contributor

vtjnash commented Oct 20, 2022

An IOStream and IOBuffer are incompatible types

@ViralBShah
Copy link
Contributor

So close this?

Comment on lines 116 to +119
function Base.convert(::Type{IOStream}, zmsg::Message)
s = IOBuffer()
write(s, zmsg)
seek(s, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function Base.convert(::Type{IOStream}, zmsg::Message)
s = IOBuffer()
write(s, zmsg)
seek(s, 0)
function Base.convert(::Union{Type{IO},Type{IOBuffer}}, zmsg::Message)
s = IOBuffer()
write(s, zmsg)
seekstart(s)

@vtjnash
Copy link
Contributor

vtjnash commented Oct 20, 2022

I am not sure, but convert seems like an odd predicate here. IOBuffer usually takes things like Message as an argument to the constructor to make an "IO-like" view of something

@JamesWrigley
Copy link
Member

I'm with @vtjnash here, it seems like a bug (or at least a footgun) that convert(IOStream, msg) returns an IOBuffer. I would suggest that we:

  1. Deprecate the Base.convert(::IOStream, ::Message) method entirely since it's just not doing the right thing.
  2. Document that people can call IOBuffer(msg) to create a zero-copy IOBuffer of a Message. This works out of the box because Message <: AbstractArray.

@ikirill, are you able to finish the PR? I can if not.

@ViralBShah
Copy link
Contributor

@ikirill doesn't seem to be too active in the last year.

@ikirill
Copy link
Author

ikirill commented Jul 20, 2024

Hi!

I don't remember anything about this PR at this point, and I gave up on trying to use ZeroMQ a very long time ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants